-
Notifications
You must be signed in to change notification settings - Fork 170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Checklist points and card points update #79
base: master
Are you sure you want to change the base?
Conversation
I used @mgan59 's commit as the base to submit a new pull request that tries to accomplish all the things attempted to fullfill in this pull request #22. It was heavily inspired by #22 but a LOT of changes were necessary to make it work But now you can also:
|
Screenshot of it in action: This PR also adds the consumed-points picker to the card title, which was in another PR but which hadn't been merged. |
Consumed-points picker for card title is not in master? I thought it was: Or am I missing something? :) |
Awesome, yeah it is in master. I guess it hadn't been pushed until today? I didn't have it on this computer yesterday, unless I'm mistaken. Cool beans :) |
Any idea on how this pull request could be improved? |
This was a mistake of the original code from pull request Q42#22. We were iterating on pointSeq (Fibonacci numbers). Actually we were iterating on the indices of this array, so we were stopping at the 9th item of the list always. Now we are iterating in all checklists items.
302417b
to
b5210cb
Compare
@oniltonmaciel it works really well. I think the only question is conceptual (whether it decreases the simplicity) @jkaizer: what do you think? If it's potentially too much, maybe we could make it togglable behind a Setting? |
Awesome work! I am thinking this should be behind a setting where the default setting disables this functionality. |
@oniltonmaciel Been on vacation and traveling, this looks cool. Will pull it down and give it a try. Cheers! |
+1 and look forward to seeing this to be published |
This still had a few edge-cases if I remember correctly. Perhaps we should QA & Polish it a bit, then hide it behind a setting? I just noticed that one of the github Issues for our project, is actually a bug in this PR rather than in the released code: #82 |
No description provided.